feat: fort map-data consumer — gyms, stations & pokestops - with DNF filtering#1228
feat: fort map-data consumer — gyms, stations & pokestops - with DNF filtering#1228jfberry wants to merge 20 commits into
Conversation
Validating in a test environmentScope: this branch routes only gyms through Golbat ( 1. Golbat prereqs — deploy #385, curl -H "X-Golbat-Secret: $S" "$G/api/gym/available"
curl -H "X-Golbat-Secret: $S" -H 'Content-Type: application/json' -XPOST "$G/api/gym/scan" \
-d '{"min":{"latitude":LAT1,"longitude":LON1},"max":{"latitude":LAT2,"longitude":LON2},"limit":1000,"filters":[]}'
curl -H "X-Golbat-Secret: $S" "$G/api/gym/id/<gym-id>"2. ReactMap — 3. Config — add 4. Confirm the endpoint path (logs):
5. Functional — gym + raid markers render; a popup shows team/slots/defenders and raid boss + moves + CP; the filter drawer populates teams, tiers ( 6. Golden parity (endpoint vs SQL): 7. Fallback — stop Golbat or set 8. Area restrictions — as an area-restricted user, confirm gyms appear only within permitted areas (exercises the new client-side |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…st-layer availability fix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dadba73 to
af62c30
Compare
… badge poison Review findings: - tier-override mode matches raid_level alone (curated boss/egg keys under-returned tier raids on endpoint sources) - b<display_type> keys move to the onlyEventStops gate (secondaryFilter's events branch consumes them, not the invasions branch) - endpoint rows resolve the quest layer as dual-capable (pure-endpoint ctx flags made effectiveQuestLayer resolve to 'both') - poison on onlyGymBadges only: onlyBadge defaults to 'all' for every user with the gymBadges perm and was silently disabling gym DNF entirely Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the all-gyms/ex/in-battle poison with real narrowing: every gym-layer display requires the team/slot match (hasGym = enabler && (team || slot)), so team_id/available_slots clauses mirroring finalTeams/finalSlots are a tight superset for all four enablers — ex/ar/in-battle narrowing stays residual. The standalone is_ar_scan_eligible clause is subsumed (ar-shown gyms also need the team match). Badge viewing still poisons. Power-up narrowing is gone for good — power-ups are no longer in the game. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This is now in my production and I have verified the DNFs through manual testing |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7984d01ba4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…/ttl Availability refresh fired a full Golbat fort-cache walk per fort type per trigger, and queryOnSessionInit triggers on EVERY page load — observed as duplicate ~110ms available-pokestops builds 200ms apart and gym rebuilds every 30-100s on a large instance. Two layers of fix: - EventManager.setAvailable gains single-flight (concurrent triggers share one refresh) and a TTL (api.availableRefreshSeconds, default 60): repeat session-init triggers within the window reuse the last result. Map markers never depend on availability — only the filter drawer's option list — so staleness is bounded and cosmetic. Scheduled intervals and the explicit /api/v1/available route force-refresh. - The three fort models' getAvailable share one GET /api/fort/available per endpoint per 30s window (Golbat builds all three sections in a single cache pass), falling back to the per-type endpoints when the combined one is unavailable. A refresh batch now costs one walk instead of three. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b2c0d62 to
c107828
Compare
…pe fallback
ReactMap always runs against a current Golbat that serves /api/fort/available,
so the per-type /api/{gym,pokestop,station}/available fallback is dead weight.
Each model's getAvailable now reads its slice of the combined result directly;
a combined failure still falls through to the SQL block (the real degradation
path for dual sources). Also drops the combined-OK diagnostic log now that the
Golbat side emits one clean "available-forts built" line.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Golbat no longer aggregates gym team/slot (every combination exists on a live instance and a claimed gym always has >=1 defender). buildGyms already generates every t/g filter key statically from the masterfile, so nothing is lost: the mapper and builder now take only the dynamic raid keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b39d377 to
ab17549
Compare
Two endpoint-backed-source gaps the SQL path handled but the endpoint path did not: - Pokestop getAll (mem): the endpoint path mapped rows + filterRTree only, so the SQL freshness gate (hideOldPokestops) and onlyLevels/power_up_level gate were never applied and secondaryFilter has no equivalent — endpoint sources rendered stale/wrong-level stops the SQL source suppresses. Gyms already mirror this (Gym.js secondaryFilter push); pokestops now do too. (power-ups are out of the game, so the onlyLevels mirror is vestigial but exact.) - Station.getDynamaxMons always ran this.query().findById, which throws on a pure-endpoint (unbound) station source, rejecting the dynamax popup. It now reads stationed_pokemon from the whole-record by-id endpoint when the source has one, falling back to SQL for dual/SQL sources. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Both Codex findings addressed in f161a96:
|
…ead code
- Delete unused Pokestop.evalQuery and its now-orphaned fs/path/fetchJson
imports (was speculative, never called).
- Extract fetchFortById() — the repeated GET-by-id + lat/lon validation used
by 5 gym/pokestop/station call sites; helper does not catch, so each caller
keeps its own swallow (manual-id miss) vs log.warn (getOne) behaviour.
- Extract parseIdFormPair() shared by the three DNF builders' wildcard-form
parses (gym raid boss, station battle pokemon, pokestop contest).
- Fix stale post-refactor comments/logs: the removed per-type availability
fallback (callers fall through to SQL, not per-type) and getAvailable log
strings that named /api/{gym,pokestop,station}/available instead of the
combined /api/fort/available.
- Minor: Object.keys over Object.entries([key]); return clauses over
clauses.length ? clauses : [].
Behaviour-preserving; eslint clean, filter builders verified by direct exercise.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR routes fort map data (gyms, pokestops, stations) through Golbat’s in-memory fort endpoints when a scanner source has an endpoint, using DNF filter translation to narrow results server-side, while preserving SQL fallback for dual sources.
Changes:
- Add endpoint-backed
getAll/getOne/getAvailablepaths for gyms, pokestops, and stations (with graceful fallback behavior and deep-link/manual-id parity). - Introduce pure mappers and DNF backends to keep key semantics consistent with existing SQL-derived filtering and
secondaryFilterbehavior. - Reduce load via (a) combined
/api/fort/availablecaching and (b) single-flight + TTL throttling forsetAvailablerefreshes.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/utils/fortAvailable.js | Adds combined /api/fort/available fetch with short-window promise caching. |
| server/src/utils/evalScannerQuery.js | Shared endpoint-or-knex query evaluator + response-shape diagnostics + by-id helper. |
| server/src/services/EventManager.js | Adds single-flight + TTL behavior for setAvailable, updates interval/route callers to force refresh. |
| server/src/services/DbManager.js | Supports dual sources (endpoint + DB) by overlaying endpoint context onto DB-derived schema context. |
| server/src/routes/api/v1/available.js | Forces refresh on PUT route to bypass TTL throttling. |
| server/src/models/stationAvailableMapper.js | Pure mapper for station availability keys. |
| server/src/models/Station.js | Adds endpoint-backed station scan (DNF + residual gates), by-id support for dynamax mons, and combined availability consumption. |
| server/src/models/pokestopScanMapper.js | Pure mapper from Golbat pokestop scan rows to secondaryFilter-compatible shape. |
| server/src/models/pokestopAvailableMapper.js | Pure mapper for pokestop availability → filter keys/conditions (SQL parity). |
| server/src/models/Pokestop.js | Adds endpoint-backed scan/getOne/getAvailable + DNF narrowing logs; factors rocket fallback; makes rewards parsing accept native JSON. |
| server/src/models/gymAvailableMapper.js | Pure mapper for gym availability raid keys. |
| server/src/models/Gym.js | Adds endpoint-backed scan/getOne/getAvailable with DNF narrowing logs and combined availability consumption. |
| server/src/filters/fort/describeDnfNarrowing.js | Shared DNF observability log formatter. |
| server/src/filters/fort/gym.js | Translates gym filters into Golbat DNF clauses. |
| server/src/filters/fort/pokestop.js | Translates pokestop filters into Golbat DNF clauses (with careful key semantics). |
| server/src/filters/fort/station.js | Translates station filters into Golbat DNF clauses (active-only narrowing where safe). |
| server/src/filters/fort/parseIdForm.js | Shared id/form parsing helper for DNF builders. |
| server/src/filters/builder/gym.js | Stops treating t/g keys as dynamic “available”; availability now only contributes raid keys. |
| packages/types/lib/server.d.ts | Adds httpAuth to DbContext and types for pokestop availability tuples; extends schema types for dual sources. |
| packages/logger/lib/tags.js | Adds TAGS.stations. |
| config/default.json | Adds api.availableRefreshSeconds. |
| docs/superpowers/plans/*.md | Adds implementation plans / design documentation for the fort consumer and DNF filtering work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function getCombinedFortAvailable(tag, mem, secret, httpAuth) { | ||
| const entry = combinedCache.get(mem) | ||
| if (entry && Date.now() - entry.ts < CACHE_MS) return entry.promise | ||
| const promise = (async () => { |
| log.warn( | ||
| TAGS.gyms, | ||
| `[FORT] combined ${mem}/api/fort/available unusable — ${describeScannerResponse(res)} — callers fall through to SQL`, | ||
| ) | ||
| } catch (e) { | ||
| log.warn( | ||
| TAGS.gyms, | ||
| `[FORT] combined ${mem}/api/fort/available error — callers fall through to SQL: ${e}`, | ||
| ) |
| if (quest.quest_reward_type) { | ||
| const rewards = JSON.parse(quest.quest_rewards) | ||
| const rewards = | ||
| typeof quest.quest_rewards === 'string' | ||
| ? JSON.parse(quest.quest_rewards) | ||
| : quest.quest_rewards | ||
| let { info } = rewards[0] |
| log.warn( | ||
| TAGS.gyms, | ||
| `[GYM] combined /api/fort/available had no gyms section — returning empty available for this endpoint source`, | ||
| ) |
| log.warn( | ||
| TAGS.stations, | ||
| `[STATION] /api/fort/available gave no battles — ${describeScannerResponse(res)} — returning empty available for this endpoint source`, | ||
| ) |
| log.warn( | ||
| TAGS.pokestops, | ||
| '[POKESTOP] /api/fort/available unavailable (e.g. fort_in_memory off) — returning empty available for this endpoint source', | ||
| ) |
|
The endpoint-backed fort paths contain an access-control bypass and several contract, filtering, and fallback regressions. These can expose restricted station data or omit valid stations, quests, and invasions despite successful lint and build checks. Full review comments:
|
- getCombinedFortAvailable: key the dedup cache by endpoint URL AND credentials, so two sources sharing a URL with different secret/httpAuth don't share a response or a cached auth failure. - Log the combined-availability warnings under the caller's tag instead of a hardcoded TAGS.gyms (pokestop/station failures no longer show as gyms). - parseRdmRewards: guard against a malformed endpoint row carrying quest_reward_type with no rewards array (was an unguarded rewards[0]). - Reword the getAvailable 'returning empty' warnings: on a dual source the code falls through to SQL, so it is empty only for a pure-endpoint source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pgap Golbat WatWowMap#382 now decodes TEMP_EVO_BRANCH_RESOURCE (reward type 20 — temp-evo branch = mega energy) into info.pokemon_id/amount, filling the generated quest_pokemon_id/quest_reward_amount columns exactly like a MEGA_RESOURCE (type 12) reward. So type 20 is now handled as mega everywhere: - SQL getAll: mega matching broadened to quest_reward_type IN (12, 20). - SQL getAvailable: queries.mega/megaAlt advertise (12, 20); type 20 excluded from the u<type> fallback set; megaBranchFallback machinery removed. - SQL search: a 'mega' reward-type search also matches type 20. - parseRdmRewards: any type-20 reward carrying info.pokemon_id is normalized to type 12 so the mega branch keys it as m<id>-<amount> (sourceQuestRewardType still preserves a u20 filter). - Endpoint DNF (buildPokestopDnfFilters): mega clauses emit quest_reward_type [12, 20] so Golbat returns type-20 mega stops. The empty-info GoFest 2026 Mewtwo stopgap (applyGoFest2026MewtwoRewardFallback + hardcoded m150-150) is retired — it required empty info and stops matching once Golbat WatWowMap#382 populates it. Requires deployed Golbat >= PR WatWowMap#382. Addresses Mygod review comment WatWowMap#3 (type-20 mega retrievability). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The endpoint (in-memory) getAll paths filter rows with filterRTree, which returns true for empty area inputs — so unlike the SQL path's getAreaSql it did NOT deny a user who has no assigned areas while strictAreaRestrictions is on and restrictions are configured. Such a user received every fort in the viewport (access-control bypass). Add areaRestrictionsDenyAll (mirroring getAreaSql's strict-deny and empty-consolidated deny) and short-circuit each fort model's endpoint branch to [] before accepting rows. Addresses Mygod review comment WatWowMap#1 (P1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…resilience Three endpoint-source correctness fixes from the Mygod review: - WatWowMap#4 (P1): a pure-endpoint pokestop source now reports hasConfirmed:true. The Golbat scan always returns confirmed incident data, but no DB schema check runs for a knex-less source, so onlyConfirmed and confirmed rocket-reward (a) filters were silently degrading to the grunt possible-encounter pool. - WatWowMap#6 (P2): historicalRarity now skips only pure-endpoint sources (no bound knex), not dual sources. Testing source.mem alone dropped a dual source DB and cleared the historical rarity map on every refresh. - WatWowMap#8 (P2): DbManager.getOne uses Promise.allSettled so a pure-endpoint source whose by-id fetch misses (falling through to an unbound this.query() that throws) no longer fails the whole single-fort lookup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Golbat now exposes confirmed invasion slots 2/3 in /api/fort/available. Read them in the availability mapper, adding an a<id>-<form> key per slot the event config marks as a reward (second/thirdReward) — matching the SQL path, which already advertises all three confirmed slots. Leaders/Giovanni (41-44) stay excluded. Addresses Mygod review comment WatWowMap#9 (P2). Requires Golbat with the slots-2/3 availability change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the thorough review, @Mygod. Walking through all nine — seven are addressed (across this PR and the Golbat companion #385), and two look like they were reviewed against a Golbat without #385's changes. Details below. Fixed
Not reproduced (appear to predate Golbat #385)
All changes build/lint clean; Golbat decoder tests pass (incl. new slots-2/3 coverage). #3, #9, and the type-20 path assume a deployed Golbat ≥ #385 + #382. |
|
I've taken the change to support slot2/slot3 availability and querying (which I previously had not added because these fields are always empty). Just like AR searching, these fields are legacy fields at the moment - but slot2/3 data coming back is something we would hope for though unlikely, whereas AR is something which is likely gone forever. A focused change later to drop AR data capture & reporting will be a wider impact in golbat |
What this does
Routes fort map data — gyms, pokestops, and stations (
getAll,getOne,getAvailable) — through Golbat's in-memory fort API (UnownHash/Golbat#385) when a scanner source has anendpoint, with transparent SQL fallback. Map pans stop issuing fort SQL entirely on endpoint-backed sources, and Golbat's DNF filtering returns only the forts that will render.Architecture
memset on the source):POST /api/<type>/scan→ per-record pure mapper → the existing, unchangedsecondaryFilter. On any failure (503 / network / bad shape) the model logs and falls through to SQL — a dual source degrades gracefully.secondaryFilterguarantees exactness. Pure backends (server/src/filters/fort/*) translate the map filters into Golbat DNF clauses (OR across clauses, AND within). The one hard invariant is never under-return; anything inexpressible (quest title/target conditions, gender, gym badges, time-window cutoffs) poisons to match-all or stays a client-side residual.DNF(n): X matched -> Y after secondaryFilter (-residual) | <clause shapes>), so any translation gap is visible in production logs.onlyManualIdfort is fetched via the by-id endpoint and joins the candidate set, mirroring SQL's(bbox) OR id = ?.Verified live (dual source, endpoint vs SQL)
station_active, gmax)Config / deploy
fort_in_memoryon), deployed before this — unknown scan-body fields fail closed to the SQL fallback.endpoint+secret(or HTTP auth) as for pokemon; DB config unchanged (fallback + un-migrated queries).No test framework exists in this repo; verification was throwaway node goldens per pure module (mappers, DNF backends), eslint/prettier, and the live parity runs above.
🤖 Generated with Claude Code